Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display up to five image attachments instead of three. #2682

Closed
wants to merge 1 commit into from

Conversation

ianmacd
Copy link

@ianmacd ianmacd commented Feb 17, 2023

Also display the first three images serially.

We're using devices with large landscape monitors, so we should make better use of all that horizontal space.

image

Contributor checklist:

Also display the first three images serially.
@KeeJef
Copy link
Collaborator

KeeJef commented Feb 27, 2023

I've pulled your PR and tested, it doesn't seem to account for smaller window width, which will lead to some images being cut off or invisible to the user (in some cases where window width is perfectly cutting off an image)

image

I've also spoken the to UX/UI team and the general feedback was that this makes the app feel more cluttered, so i think we would err on the side of closing this unless there is significant community desire for this change.

@mdPlusPlus
Copy link

unless there is significant community desire for this change

Here to voice community desire.

@ianmacd
Copy link
Author

ianmacd commented Feb 27, 2023

I've pulled your PR and tested, it doesn't seem to account for smaller window width, which will lead to some images being cut off or invisible to the user (in some cases where window width is perfectly cutting off an image)

This PR was born of the failure of Session to account for large window sizes when displaying multiple attachments.

If you feel that this change doesn't take into account narrower window sizes, then I think we're coming to an important realisation: Neither of us knows how wide the user's Session window is, and different users will have windows of different widths. Why should any of them have to compromise?

Perhaps the best behaviour, then, is one that adapts to the amount of horizontal space available to each and every user. The more space available, the greater the number of attachments that can be shown without cluttering the screen.

@KeeJef
Copy link
Collaborator

KeeJef commented Apr 17, 2023

Closing for now, as this PR does not account for window size, there doesn't seem to be significant community desire and the UX/UI team has expressed their view that increasing the default images shown makes the application feel more cluttered, which they would like to avoid.

UI/UX decisions like this are always going to be feel fairly arbitrary, akin to judging art, its in the eye of the beholder; which designs enhance function while preserving form? so i can understand feeling annoyed. Nonetheless thank you for putting in the effort to produce this code and submit this PR.

@KeeJef KeeJef closed this Apr 17, 2023
@mdPlusPlus
Copy link

there doesn't seem to be significant community desire

Go ask in the image focused communities about that. The people who would support that cause are simply not aware of it, because they aren't active on GitHub.

@majestrate
Copy link

majestrate commented Apr 17, 2023

Closing for now, as this PR does not account for window size, there doesn't seem to be significant community desire and the UX/UI team has expressed their view that increasing the default images shown makes the application feel more cluttered, which they would like to avoid.

the fact that he made this PR is proof that there is a very notable non trivial amount of community desire.

this needs to be reopened and our process in how we do UI/UX revised to include the community as the primary entity involved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants